Update documentation with 18 changed files (#4474) - #4487

Step 17b — Comprehensive Code Review ✅ Approve (non-blocking nits)

Reviewed the full diff for #4474 (label self-heal for the OODA no-progress escalation). The core fix is well-designed, correct, and thoroughly tested. Summary below.

Verdict: Approve. No blocking issues. Two low-severity nits + one accepted edge case.

What's good

• Correct fail-safe design. ensure_label → LabelDisposition never errors: Attach on success/idempotent-exists, Omit{reason} on any failure, so a blocked goal is always escalated (issue filed with or without the label). This is exactly the requirement.
• Idempotency. Case-insensitive "already exists" stderr match treats the non-zero "label already present" exit as success. Existing one-issue-per-stall dedup (already_tracked) is untouched.
• Single source of truth for label argv. LabelDisposition::label_args() + per-site *_issue_argv builders keep --label splicing consistent across all three filer sites.
• No silent fallbacks. Every degrade path emits structured tracing::warn! (targets simard::ooda / simard::ooda_brain per site); open_tracking_issue was upgraded from .status() (which swallowed non-zero exits) to .output() with a surfaced, bounded failure note — fixing the latent second bug.
• Injectable seams. LabelEnsureExecutor fn-pointer mirrors the proven CreateIssueExecutor pattern; the three failure classes (create-ok, already-exists, unauthorized, spawn-fail, huge-stderr) are all unit-tested without real gh, plus real-subprocess tests via fake_gh.
• Hygiene. No new print!/println!, no TODOs/stubs/unimplemented!; all unwrap()/panic! are test-only. Ambient repo scoping (no -R) documented so gh label create and gh issue create can't target different repos.

Nits (non-blocking)

N1 — truncate_for_log is now a third copy. This PR adds gh_client::truncate_for_log (2048 B cap) alongside the pre-existing spawn.rs local (256 B) and ooda_brain::rustyclawd (8 KiB) copies. It's consistent with the repo's existing per-module convention, so not blocking — but note the two escalation failure-note paths now truncate at different bounds: spawn::tracking_issue_failure_note uses the 256 B local helper, while gh_client::ensure_label_with uses the 2048 B one. Worth a future consolidation into one shared helper (e.g. a logfmt util) with a single agreed cap.

N2 — Over-broad visibility. gh_client::truncate_for_log and GH_STDERR_LOG_LIMIT are pub(crate) but only referenced within gh_client. Could be module-private (fn / const) to keep the surface minimal. Trivial.

Accepted edge case (no action needed)

• TOCTOU between gh label create and gh issue create. If the label were deleted in the sub-second window after ensure_label returns Attach, gh issue create --label would exit non-zero. This is surfaced (not swallowed) via the Ok(out) non-success branches and is astronomically unlikely; documenting here for completeness rather than as a defect.
• Pre-existing: the no_progress::file_issue Ok(out) error branch logs gh stderr unbounded (stderr = %String::from_utf8_lossy(...)), unlike the new bounded paths. Pre-existing code, out of scope for this PR, but a candidate to route through the same truncation for consistency.

Checklist

• Code quality and standards — idiomatic, well-documented, matches existing patterns
• Test coverage adequate — create / already-exists / unauthorized / spawn-fail / huge-stderr / degrade + real-subprocess argv, at all three sites
• No TODOs, stubs, or swallowed exceptions — .status()→.output() fix removes the swallow
• No unimplemented functions
• Logic correctness — never-error contract verified; idempotent-exists handled
• Edge case handling — bounded logs, UTF-8-safe truncation, degrade-not-fail; TOCTOU noted as accepted

Build + full suite were green in Step 11b (9219 passed; 0 failed). LGTM to merge.

Step 17c — Security Review ✅ No high-confidence vulnerabilities

Threat-modeled the full diff for #4474 (label self-heal for OODA no-progress escalation) across the three source files. The change is defensively designed; no blocking findings.

Checklist

• Security requirements met — fail-safe degrade preserves escalation; no new privileged surface
• No new vulnerabilities — see analysis below
• Sensitive-data handling — no tokens/secrets read, logged, or passed anywhere in the diff
• AuthN/AuthZ — relies on gh CLI ambient auth; degrades correctly on insufficient scope
• Injection — argv-only subprocess execution; no shell, no interpolation

Analysis

1. Command / shell injection — SAFE.
Every gh call is Command::new("gh").args(&argv) — a real argv vector, never sh -c/string interpolation (confirmed: no sh -c, /bin/sh, or Command::new("sh") in production diff). title/body are passed as discrete argv elements (--title <title>, --body <body>/--body-file -), so goal-derived content containing shell metacharacters (;, $(), backticks) is delivered literally and never interpreted.

2. Label injection — SAFE.
OODA_STUCK_LABEL is a compile-time constant ("ooda-stuck"). label_args splices ["--label", "ooda-stuck"] unconditionally or nothing — no attacker-controlled label value ever reaches argv.

3. Sensitive-data handling — SAFE.
No GH_TOKEN/GITHUB_TOKEN/.env()/credential references in the diff (confirmed by grep). Auth is entirely delegated to the gh binary's own credential store; the code never touches the token. Logged fields are limited to goal_id, the constant label, truncated gh stderr, title, issue number, and degrade reason — no secret material.

4. Log-flood / DoS — MITIGATED.
GH_STDERR_LOG_LIMIT = 2048 + truncate_for_log bound hostile/runaway gh stderr on a UTF-8 boundary before it is folded into a degrade reason or tracing field. Explicitly tested (label_huge_stderr injects 100 KB → truncated). Good defensive control.

5. AuthZ degrade — SAFE (fail-safe, no priv-esc).
A token with issue-write but not label-write does not fail the escalation: ensure_label returns LabelDisposition::Omit and the issue is still filed (without the label), with the reason surfaced via tracing::warn. Non-zero exits and spawn errors are logged (tracing::error), never silently swallowed — the .status()→.output() upgrade is the mechanism. No privilege escalation path is introduced.

Non-blocking observations (accepted / out of scope)

• Log-injection (low): goal-derived title and gh stderr are logged with % formatting. In a plaintext log sink, embedded newlines could forge log lines. Pre-existing pattern, structured tracing fields, low severity — no action required here.
• TOCTOU (low): a label deleted between ensure_label and gh issue create re-triggers the same missing-label failure, which the design already degrades around safely. Accepted.

Verdict: No high-confidence security findings. argv-only gh invocation, constant label, bounded stderr logging, no secret handling, and fail-safe authz degrade. Approve from a security standpoint.